Test : aggregation edge cases for list and scalar values by Achieve3318 · Pull Request #14170 · infiniflow/ragflow

Achieve3318 · 2026-04-16T14:06:34Z

This PR adds focused unit tests for aggregate_by_field in OceanBase memory utilities to improve behavior coverage for real-world input shapes.

Adds test coverage for list-valued aggregation fields, including whitespace trimming and skipping invalid list entries.
Adds test coverage for scalar field values to ensure blank/non-string values are ignored.
Confirms aggregation output remains correct and stable for mixed-quality message payloads.

Why this helps

It strengthens regression protection for aggregation logic used by memory retrieval flows, with no production code changes and minimal review risk.

coderabbitai · 2026-04-16T14:06:43Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 47ca8938-48d3-4607-9409-df387e71d4b3

📥 Commits

Reviewing files that changed from the base of the PR and between b7b8021 and e64e6a7.

📒 Files selected for processing (1)

test/unit_test/memory/utils/test_ob_conn_aggregation.py

🚧 Files skipped from review as they are similar to previous changes (1)

test/unit_test/memory/utils/test_ob_conn_aggregation.py

📝 Walkthrough

Walkthrough

Added two pytest unit tests for aggregate_by_field: one checks list-valued element trimming and per-element counting while ignoring blanks/non-strings; the other checks scalar handling by ignoring None, numeric, and blank/whitespace-only strings.

Changes

Test Coverage Expansion

Layer / File(s)	Summary
Add pytest import and new tests `test/unit_test/memory/utils/test_ob_conn_aggregation.py`	Added `pytest` import and two `@pytest.mark.p2` tests in `TestAggregateByField`: one validates list-valued aggregation (trimming elements, counting non-empty strings, ignoring blank strings and non-string elements) and the other validates scalar-field aggregation (ignores `None`, numeric, and blank/whitespace-only strings; counts only valid non-empty strings).

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 With a twitch and a tap I write a test,

Trimming strings and skipping the rest,
Lists and scalars counted right,
Blank bits tucked away from sight,
A tiny rabbit hops in delight.

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name	Status	Explanation	Resolution
Description check	⚠️ Warning	The PR description provides a clear summary of the added tests and their purpose, but lacks the required template structure with 'What problem does this PR solve?' and 'Type of change' sections.	Restructure the description to include the required template sections: 'What problem does this PR solve?' and 'Type of change' with appropriate checkbox selection.
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (3 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and specifically describes the main change: adding tests for aggregation edge cases covering both list and scalar values.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

Add focused unit tests to verify whitespace trimming and non-string filtering behavior in memory aggregation results.

Achieve3318 · 2026-04-16T14:12:44Z

Hi, @yingfeng , Could you review my PR, please?

coderabbitai

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@test/unit_test/memory/utils/test_ob_conn_aggregation.py`:
- Around line 57-74: The two new tests
test_aggregates_list_values_and_trims_whitespace and
test_ignores_non_string_and_blank_scalar_values are missing the required pytest
priority markers; add the appropriate pytest marker (e.g., `@pytest.mark.p1` or
`@pytest.mark.p2/`@pytest.mark.p3 per project guidelines) above each test function
so they include a priority marker, and ensure you import pytest if not already
present so the decorators resolve.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

Push a commit to this branch (recommended)
Create a new PR with the fixes

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 16f5e3ca-fd10-4cdf-b683-109f1fb55000

📥 Commits

Reviewing files that changed from the base of the PR and between f906a20 and b7b8021.

📒 Files selected for processing (1)

test/unit_test/memory/utils/test_ob_conn_aggregation.py

Ensure new unit tests follow test suite marker conventions for CI and reviewer checks.

Achieve3318 · 2026-04-17T02:33:40Z

Hi, @yingfeng , Could you review my PR, please?

Achieve3318 · 2026-04-19T22:55:54Z

hi, @yingfeng , Could you review my PR?

Achieve3318 · 2026-04-24T13:37:26Z

Hi, @yingfeng , Could you review my PR, please?

codecov · 2026-05-12T07:16:33Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 94.16%. Comparing base (f906a20) to head (e64e6a7).
⚠️ Report is 381 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #14170      +/-   ##
==========================================
- Coverage   96.66%   94.16%   -2.50%     
==========================================
  Files          10       10              
  Lines         690      703      +13     
  Branches      108      112       +4     
==========================================
- Hits          667      662       -5     
- Misses          8       25      +17     
- Partials       15       16       +1

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

dosubot Bot added the size:L This PR changes 100-499 lines, ignoring generated files. label Apr 16, 2026

dosubot Bot added 🐖api The modified files are located under directory 'api/apps/sdk' 🧪 test Pull requests that update test cases. labels Apr 16, 2026

Test aggregation edge cases for list and scalar field values.

b7b8021

Add focused unit tests to verify whitespace trimming and non-string filtering behavior in memory aggregation results.

Achieve3318 force-pushed the test/memory-aggregation-edge-cases branch from 7fa1917 to b7b8021 Compare April 16, 2026 14:11

dosubot Bot added size:XS This PR changes 0-9 lines, ignoring generated files. and removed size:L This PR changes 100-499 lines, ignoring generated files. labels Apr 16, 2026

coderabbitai Bot reviewed Apr 16, 2026

View reviewed changes

Comment thread test/unit_test/memory/utils/test_ob_conn_aggregation.py

Add required pytest priority markers to new aggregation tests.

e64e6a7

Ensure new unit tests follow test suite marker conventions for CI and reviewer checks.

KevinHuSh added the ci Continue Integration label May 12, 2026

KevinHuSh marked this pull request as draft May 12, 2026 07:06

KevinHuSh marked this pull request as ready for review May 12, 2026 07:06

KevinHuSh merged commit 2cc206e into infiniflow:main May 12, 2026
3 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Test : aggregation edge cases for list and scalar values#14170

Test : aggregation edge cases for list and scalar values#14170
KevinHuSh merged 2 commits into
infiniflow:mainfrom
Achieve3318:test/memory-aggregation-edge-cases

Achieve3318 commented Apr 16, 2026 •

edited

Loading

Uh oh!

coderabbitai Bot commented Apr 16, 2026 •

edited

Loading

Walkthrough

Changes

Estimated code review effort

Poem

❌ Failed checks (2 warnings)

Uh oh!

Achieve3318 commented Apr 16, 2026

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

Achieve3318 commented Apr 17, 2026

Uh oh!

Achieve3318 commented Apr 19, 2026

Uh oh!

Achieve3318 commented Apr 24, 2026

Uh oh!

codecov Bot commented May 12, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

Achieve3318 commented Apr 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Why this helps

Uh oh!

coderabbitai Bot commented Apr 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Estimated code review effort

Poem

❌ Failed checks (2 warnings)

Uh oh!

Achieve3318 commented Apr 16, 2026

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Achieve3318 commented Apr 17, 2026

Uh oh!

Achieve3318 commented Apr 19, 2026

Uh oh!

Achieve3318 commented Apr 24, 2026

Uh oh!

codecov Bot commented May 12, 2026

Codecov Report

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Achieve3318 commented Apr 16, 2026 •

edited

Loading

coderabbitai Bot commented Apr 16, 2026 •

edited

Loading